Add a stack-`pin!`-ning macro to `core::pin`. #93176

danielhenrymantilla · 2022-01-21T21:40:46Z

Tracking Issue for core::pin::pin! #93178

pin! allows pinning a value to the stack. Thanks to being implemented in the stdlib, which gives access to macro macros, and to the private .pointer field of the Pin wrapper, it was recently discovered (archive link), contrary to popular belief, that it is actually possible to implement and feature such a macro:

let foo: Pin<&mut PhantomPinned> = pin!(PhantomPinned);
stuff(foo);

or, directly:

stuff(pin!(PhantomPinned));

For context, historically, this used to require one of the two following syntaxes:

let foo = PhantomPinned;
pin!(foo);
stuff(foo);

pin! {
    let foo = PhantomPinned;
}
stuff(foo);

This macro thus allows, for instance, doing things like:

fn block_on<T>(fut: impl Future<Output = T>) -> T {
    // Pin the future so it can be polled.
-   let mut fut = Box::pin(fut);
+   let mut fut = pin!(fut);

    // Create a new context to be passed to the future.
    let t = thread::current();
    let waker = Arc::new(ThreadWaker(t)).into();
    let mut cx = Context::from_waker(&waker);

    // Run the future to completion.
    loop {
        match fut.as_mut().poll(&mut cx) {
            Poll::Ready(res) => return res,
            Poll::Pending => thread::park(),
        }
    }
}

c.f., https://doc.rust-lang.org/1.58.1/alloc/task/trait.Wake.html

And so on, and so forth.

I don't think such an API can get better than that, barring full featured language support (&pin references or something), so I see no reason not to start experimenting with featuring this in the stdlib already 🙂

cc @rust-lang/wg-async-foundations [EDIT: this doesn't seem to have pinged anybody 😩, thanks @yoshuawuyts for the real ping]

r? @joshtriplett

Docs preview

video1167605346.mp4

Implementation

The implementation ends up being dead simple (so much it's embarrassing):

pub macro pin($value:expr $(,)?) {
    Pin { pointer: &mut { $value } }
}

and voilà!

The key for it working lies in the rules governing the scope of anonymous temporaries.

Comments and context

This is Pin::new_unchecked(&mut { $value }), so, for starters, let's
review such a hypothetical macro (that any user-code could define):

macro_rules! pin {( $value:expr ) => (
    match &mut { $value } { at_value => unsafe { // Do not wrap `$value` in an `unsafe` block.
        $crate::pin::Pin::<&mut _>::new_unchecked(at_value)
    }}
)}

Safety:

type P = &mut _. There are thus no pathological Deref{,Mut} impls that would break Pin's invariants.
{ $value } is braced, making it a block expression, thus moving the given $value, and making it become an anonymous temporary.
By virtue of being anonynomous, it can no longer be accessed, thus preventing any attemps to mem::replace it or mem::forget it, etc.

This gives us a pin! definition that is sound, and which works, but only in certain scenarios:

If the pin!(value) expression is directly fed to a function call:
let poll = pin!(fut).poll(cx);

If the pin!(value) expression is part of a scrutinee:

match pin!(fut) { pinned_fut => {
    pinned_fut.as_mut().poll(...);
    pinned_fut.as_mut().poll(...);
}} // <- `fut` is dropped here.

Alas, it doesn't work for the more straight-forward use-case: let bindings.

let pinned_fut = pin!(fut); // <- temporary value is freed at the end of this statement
pinned_fut.poll(...) // error[E0716]: temporary value dropped while borrowed
                     // note: consider using a `let` binding to create a longer lived value

Issues such as this one are the ones motivating RFC: Better temporary lifetimes (so e.g. .as_slice() works) rfcs#66

This makes such a macro incredibly unergonomic in practice, and the reason most macros out there had to take the path of being a statement/binding macro (e.g., pin!(future);) instead of featuring the more intuitive ergonomics of an expression macro.

Luckily, there is a way to avoid the problem. Indeed, the problem stems from the fact that a temporary is dropped at the end of its enclosing statement when it is part of the parameters given to function call, which has precisely been the case with our Pin::new_unchecked()!

For instance,

let p = Pin::new_unchecked(&mut <temporary>);

becomes:

let p = { let mut anon = <temporary>; &mut anon };

However, when using a literal braced struct to construct the value, references to temporaries can then be taken. This makes Rust change the lifespan of such temporaries so that they are, instead, dropped at the end of the enscoping block.

For instance,

let p = Pin { pointer: &mut <temporary> };

becomes:

let mut anon = <temporary>;
let p = Pin { pointer: &mut anon };

which is exactly what we want.

Finally, we don't hit problems w.r.t. the privacy of the pointer field, or the unqualified Pin name, thanks to decl_macros being fully hygienic (def_site hygiene).

TODO

Add compile-fail tests with attempts to break the Pin invariants thanks to the macro (e.g., try to access the private .pointer field, or see what happens if such a pin is used outside its enscoping scope (borrow error));
Follow-up stuff:
- Try to experiment with adding pin! to the prelude: this may require to be handled with some extra care, as it may lead to issues reminiscent of those of assert_matches!: Is there a gentler way to land the assert_matches macro? #82913
- Create the tracking issue.

danielhenrymantilla · 2022-01-21T21:43:47Z

@rustbot modify labels: +T-libs-api +A-async-await +F-decl_macro +A-pin

ibraheemdev

👍, but I think this deserves a beginner-friendly explanation on top of why this is needed/useful, e.g: https://docs.rs/tokio/latest/tokio/macro.pin.html. Also, this needs a tracking issue.

yoshuawuyts · 2022-01-21T22:26:24Z

cc @rust-lang/wg-async-foundations

cramertj · 2022-01-21T22:47:11Z

What led to the decision to call this pin! rather than pin_mut!? I'm aware of inconsistency here between futures and tokio, and it would be good to resolve.

Personally, I think that since there are known use-cases for Pin<&T>, it seems like it would be valuable to offer both pin_ref and pin_mut, as is standard in other library APIs.

cramertj · 2022-01-21T22:49:07Z

(That said, it also requires move-access to a value in order to use this macro even in the &-case, and the resulting Pin<&mut T> can be converted into a Pin<&T>, so I'd understand only offering this one.)

danielhenrymantilla · 2022-01-21T23:13:42Z

@cramertj Right now I just followed the more pervasive name out there, but I have no opinions on this matter whatsoever.
In a way, I even prefer pin_mut!, but I have to admit I like the look of that s/Box::pin/pin!/diff 😄. Ideally we'd have had something like <&mut _>::pin! or mut_ref::pin!, so as to be consistent with Box::pin naming, but lacking that, I can see the appeal of pin_mut!.

FWIW, now that the macro is so ergonomic, there is an argument to be made for pin_ref! (besides sheer symmetry):
let x = pin_mut!(…).as_ref(); will yield a "dangling"/unusable x, because the .as_ref() then breaks the temporary promotion.

So with all that being said, I personally lean ever so slightly towards pin_mut!, but let's see what others think on the matter. It's something easy to change before the stabilization anyways. Lemme add the concern to the tracking issue.

eddyb · 2022-01-22T00:05:57Z

cc @nikomatsakis This could maybe use a more direct reference to the let RHS temporaries "scope extension" rules, but I forget where this is described, the best I can find is a section in the reference.

danielhenrymantilla · 2022-01-22T00:16:13Z

@eddyb agreed, I searched among the RFCs and surprisingly there was no normative stuff. But that reference link you provided is great! I'll add a permalink to it to the comments tomorrow

danielhenrymantilla · 2022-01-22T00:18:16Z

The exact rules for temporary lifetime extension are subject to change. This is describing the current behavior only.

Hmm, isn't it super-hard for these rules to change? Both reducing and enlarging the scope of temporaries can break code out there, both at compile-time (if borrows are involved) or at runtime (if lock guards are involved).

eholk · 2022-01-22T00:39:21Z

This looks awesome!

I suppose I'd have a slight preference towards pin_mut! as well, for symmetry with similar APIs, but I don't feel strongly. Having a short name like pin! is also nice.

taiki-e · 2022-01-22T04:16:18Z

I would prefer pin! over pin_mut!. (I've considered its renaming in the past in pin-utils and futures: rust-lang/pin-utils#32)

Considering the stack pinning utilities for shared references are unneeded, we probably don't need mut of pin_mut! macro.

Darksonn · 2022-01-22T09:23:12Z

I know that people sometimes claim that Pin<&T> is useful in some cases, but I have yet to see an actual use-case. I support the name pin!.

library/core/src/pin.rs

Co-Authored-By: Mara Bos <[email protected]>

Since `decl_macro`s and/or `Span::def_site()` is deemed quite unstable, no public-facing macro that relies on it can hope to be, itself, stabilized. We circumvent the issue by no longer relying on field privacy for safety and, instead, relying on an unstable feature-gate to act as the gate keeper for non users of the macro (thanks to `allow_internal_unstable`). This is technically not correct (since a `nightly` user could technically enable the feature and cause unsoundness with it); or, in other words, this makes the feature-gate used to gate the access to the field be (technically unsound, and in practice) `unsafe`. Hence it having `unsafe` in its name. Back to the macro, we go back to `macro_rules!` / `mixed_site()`-span rules thanks to declaring the `decl_macro` as `semitransparent`, which is a hack to basically have `pub macro_rules!` Co-Authored-By: Mara Bos <[email protected]>

This thus still makes it technically possible to enable the feature, and thus to trigger UB without `unsafe`, but this is fine since incomplete features are known to be potentially unsound (labelled "may not be safe"). This follows from the discussion at rust-lang#93176 (comment)

Addresses https://github.com/rust-lang/rust/pull/93176/files#r795258110.

compiler/rustc_feature/src/active.rs

m-ou-se · 2022-02-14T19:17:52Z

@bors r+

bors · 2022-02-14T19:17:53Z

📌 Commit bf2a9dc has been approved by m-ou-se

bors · 2022-02-15T09:32:07Z

⌛ Testing commit bf2a9dc with merge 6421a49...

bors · 2022-02-15T11:59:31Z

☀️ Test successful - checks-actions
Approved by: m-ou-se
Pushing 6421a49 to master...

rust-timer · 2022-02-15T13:31:17Z

Finished benchmarking commit (6421a49): comparison url.

Summary: This benchmark run did not return any relevant results.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

`core::pin::pin!` was unstably added by [rust-lang/rust#93176][1]. This replaces `pin_utils::pin_mut!`. [1]: rust-lang/rust#93176

…, r=Amanieu Rename `pointer` field on `Pin` A few days ago, I was helping another user create a self-referential type using `PhantomPinned`. However, I noticed an odd behavior when I tried to access one of the type's fields via `Pin`'s `Deref` impl: ```rust use std::{marker::PhantomPinned, ptr}; struct Pinned { data: i32, pointer: *const i32, _pin: PhantomPinned, } fn main() { let mut b = Box::pin(Pinned { data: 42, pointer: ptr::null(), _pin: PhantomPinned, }); { let pinned = unsafe { b.as_mut().get_unchecked_mut() }; pinned.pointer = &pinned.data; } println!("{}", unsafe { *b.pointer }); } ``` ```rust error[E0658]: use of unstable library feature 'unsafe_pin_internals' --> <source>:19:30 | 19 | println!("{}", unsafe { *b.pointer }); | ^^^^^^^^^ error[E0277]: `Pinned` doesn't implement `std::fmt::Display` --> <source>:19:20 | 19 | println!("{}", unsafe { *b.pointer }); | ^^^^^^^^^^^^^^^^^^^^^ `Pinned` cannot be formatted with the default formatter | = help: the trait `std::fmt::Display` is not implemented for `Pinned` = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead = note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info) ``` Since the user named their field `pointer`, it conflicts with the `pointer` field on `Pin`, which is public but unstable since Rust 1.60.0 with rust-lang#93176. On versions from 1.33.0 to 1.59.0, where the field on `Pin` is private, this program compiles and prints `42` as expected. To avoid this confusing behavior, this PR renames `pointer` to `__pointer`, so that it's less likely to conflict with a `pointer` field on the underlying type, as accessed through the `Deref` impl. This is technically a breaking change for anyone who names their field `__pointer` on the inner type; if this is undesirable, it could be renamed to something more longwinded. It's also a nightly breaking change for any external users of `unsafe_pin_internals`.

…, r=Amanieu,dtolnay Rename `pointer` field on `Pin` A few days ago, I was helping another user create a self-referential type using `PhantomPinned`. However, I noticed an odd behavior when I tried to access one of the type's fields via `Pin`'s `Deref` impl: ```rust use std::{marker::PhantomPinned, ptr}; struct Pinned { data: i32, pointer: *const i32, _pin: PhantomPinned, } fn main() { let mut b = Box::pin(Pinned { data: 42, pointer: ptr::null(), _pin: PhantomPinned, }); { let pinned = unsafe { b.as_mut().get_unchecked_mut() }; pinned.pointer = &pinned.data; } println!("{}", unsafe { *b.pointer }); } ``` ```rust error[E0658]: use of unstable library feature 'unsafe_pin_internals' --> <source>:19:30 | 19 | println!("{}", unsafe { *b.pointer }); | ^^^^^^^^^ error[E0277]: `Pinned` doesn't implement `std::fmt::Display` --> <source>:19:20 | 19 | println!("{}", unsafe { *b.pointer }); | ^^^^^^^^^^^^^^^^^^^^^ `Pinned` cannot be formatted with the default formatter | = help: the trait `std::fmt::Display` is not implemented for `Pinned` = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead = note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info) ``` Since the user named their field `pointer`, it conflicts with the `pointer` field on `Pin`, which is public but unstable since Rust 1.60.0 with rust-lang#93176. On versions from 1.33.0 to 1.59.0, where the field on `Pin` is private, this program compiles and prints `42` as expected. To avoid this confusing behavior, this PR renames `pointer` to `__pointer`, so that it's less likely to conflict with a `pointer` field on the underlying type, as accessed through the `Deref` impl. This is technically a breaking change for anyone who names their field `__pointer` on the inner type; if this is undesirable, it could be renamed to something more longwinded. It's also a nightly breaking change for any external users of `unsafe_pin_internals`.

Rollup merge of rust-lang#119562 - LegionMammal978:rename-pin-pointer, r=Amanieu,dtolnay Rename `pointer` field on `Pin` A few days ago, I was helping another user create a self-referential type using `PhantomPinned`. However, I noticed an odd behavior when I tried to access one of the type's fields via `Pin`'s `Deref` impl: ```rust use std::{marker::PhantomPinned, ptr}; struct Pinned { data: i32, pointer: *const i32, _pin: PhantomPinned, } fn main() { let mut b = Box::pin(Pinned { data: 42, pointer: ptr::null(), _pin: PhantomPinned, }); { let pinned = unsafe { b.as_mut().get_unchecked_mut() }; pinned.pointer = &pinned.data; } println!("{}", unsafe { *b.pointer }); } ``` ```rust error[E0658]: use of unstable library feature 'unsafe_pin_internals' --> <source>:19:30 | 19 | println!("{}", unsafe { *b.pointer }); | ^^^^^^^^^ error[E0277]: `Pinned` doesn't implement `std::fmt::Display` --> <source>:19:20 | 19 | println!("{}", unsafe { *b.pointer }); | ^^^^^^^^^^^^^^^^^^^^^ `Pinned` cannot be formatted with the default formatter | = help: the trait `std::fmt::Display` is not implemented for `Pinned` = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead = note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info) ``` Since the user named their field `pointer`, it conflicts with the `pointer` field on `Pin`, which is public but unstable since Rust 1.60.0 with rust-lang#93176. On versions from 1.33.0 to 1.59.0, where the field on `Pin` is private, this program compiles and prints `42` as expected. To avoid this confusing behavior, this PR renames `pointer` to `__pointer`, so that it's less likely to conflict with a `pointer` field on the underlying type, as accessed through the `Deref` impl. This is technically a breaking change for anyone who names their field `__pointer` on the inner type; if this is undesirable, it could be renamed to something more longwinded. It's also a nightly breaking change for any external users of `unsafe_pin_internals`.

rust-highfive assigned joshtriplett Jan 21, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 21, 2022

rustbot added A-async-await Area: Async & Await A-pin Area: Pin F-decl_macro `#![feature(decl_macro)]` T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jan 21, 2022